-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reorganize docs and add a quickstart guide #33
Reorganize docs and add a quickstart guide #33
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
77bf00e
to
38a8d83
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly good to me! A couple of typos here and there to fix. I think it would also be useful to add images to the quickstart guide.
docs/quickstart.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we can add some images to this file, so that people can see view cell outputs; we could imagine having this be a Jupyter notebook that we convert to Markdown using jupyter nbconvert --to markdown quickstart.ipynb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this started out as a jupyter notebook and I ended up switching it to markdown because I felt that the outputs of most of the code snippets in the quickstart guide are not essential and may even be distracting, as the snippets are meant to illustrate how to use apc
objects, not how to plot things. And of course markdown is much easier to write, review, and version control.
How do you feel about leaving this as markdown for now but adding static images to show the output of print
ing colors, palettes, and gradients? (which are code snippets whose outputs I feel it would definitely be helpful to show)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems reasonable. I think it would be useful to show the outputs of plt.show()
calls as well, since I think those would convey the expected behaviors of apc.mpl.setup()
and apc.mpl.style_plot()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added images for the color/palette/gradient swatches!
I guess I have a different feeling about adding images for the plt.show()
outputs, which is that the quickstart guide should have a "just type this and your plots will be styled" vibe rather than "here is what your plots will look like". I think I have this feeling because:
- it helps make the quickstart guide as succinct as possible.
- the expected behaviors of
setup
andstyle_plot
are already documented in thestyle_usage
notebook and may be hard to grok without the before-and-after approach that that notebook uses (and that would feel too verbose in a quickstart guide). - if we show plot outputs, the quickstart guide will start to reproduce the
style_usage
notebook.
In lieu of showing plot outputs, how would you feel about adding more obvious/explicit links to the style_usage
notebook in the quickstart guide?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose it's fine not to show the plot examples in the quickstart then, as long as we link to the relevant before/after examples in style_usage
!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added very explicit links to style_usage
to the quickstart. I think it's good enough for now; happy to revisit later!
Co-authored-by: Dennis August Sun <[email protected]> Signed-off-by: Keith Cheveralls <[email protected]>
@mezarque I made one minor change after you reviewed to add |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me for now!
…to the main readme
The documentation for the package currently consists of a series of jupyter notebooks. This PR:
docs/
subdirectory.Note: github does not recognize that the
style_usage.ipynb
notebook was moved; it treats it as a new file, so there's no way to view the diff, which is annoying. (I was careful to move the file in one commit and modify it in a second commit, so the git history "knows" about the renaming; this must be a github thing).Other changes
/plot_testing
into/docs/examples
.style_usage
notebook.style_usage
notebook for clarity/succinctness.ipykernel
from a main dependency to a dev dependency (as it is only needed to run the documentation notebooks).Style changes
These were made in discussion with Dennis.
mpl.style_axis
tostyle_plot
for clarity.lightgray
togray
to be consistent with the style guide.-rename
brown_shades
towarm_gray_shades
andgrey_shades
tocool_gray_shades
.PR checklist
conda
environments.